-
Notifications
You must be signed in to change notification settings - Fork 94
[Guide] creating a promise and continue to run steps async #85
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
I have always found that phrase confusing and bizarre, and would hope it could be replaced with something either implicit (like in the examples I give, e.g. "in x seconds, resolve p with bar"), or more like:
But this example is too contrived; why would we queue a microtask to do this? Usually we need to wait on some external input. We should perhaps examine existing examples. That said, this isn't really a battle worth fighting, probably; if people really like "return x but then magically somehow also do things after the |
The problem is that we don' have "microtasks" defined anywhere in the platform yet. In the mind of the spec writer, they are just following the assumed execution flow of ES. If you want a real example, I'm just in the middle of rewriting the following using your guide: (note that the url argument has been dropped in my rewrite, but that gives you an idea) |
Probably a bit early to use the example I gave, actually. It needs a big rewrite. |
But how does this make sense? In ES, you can't perform any steps at all after you return. That's why it's so confusing. |
Right, the unspoken assumption is that spec text is running in the WebIDL limbo... the ghastly place between ES and native code and where the dammed await judgement :) |
How about: The steps to request to add a bookmark are given by the following algorithm. The algorithm returns a promise.
|
That looks lovely. Minor points:
|
I know you are right in that I should do that - but I don't want to do the work twice. I'll add a big red note in my spec and link to the bug noting that DOMException has to extend ES Error has to be fixed before the spec can be implemented. The spec is a long way away from being implementable - so right now it's just for show and tell. I hope that's ok.
Ok, I'll do, for example "Reject promise with a new DOMException whose name is "AbortError" and return."
Agree. As above.
The problem is that ... there is no setImmidiate() yet across browsers and the other way to do it is using a bunch of hacks 😢. I hear ES7 will fix this ... or will get some magic way of getting access to the task queues (you probably know more about this, and I'm pretty sure we've discussed this before). Anyway, in W3C spec land, the closest we have to those is the concept of "queue a task". I'm going to keep that for now as it's not particularly harmful. |
Seems perfect :)
No, you misunderstood me. Think of how you would implement the above algorithm in JS. It would not be: function algorithm() {
return new Promise(function resolver() {
// Note: `resolver` is not called in the next tick; it is called immediately.
// Now "queue a task":
setImmediate(function () {
if (methodWasNotInvokedAsAResultOfExplicitUserAction()) {
reject(new DOMException("bad stuff", { name: "SecurityError" }));
return;
}
if (documentsModeOfOperationIsStandalone()) {
reject(new DOMException("bad stuff", { name: "NotSupported" }));
return;
}
var info = getWebApplicationsMetadata();
userAgentSpecificChoooser(function (result) {
if (!result) {
reject(new DOMException("bad stuff", { name: "AbortError" }));
return;
}
resolve(undefined);
});
});
});
} Instead you would just do: function algorithm() {
return new Promise(function resolver() {
// Note: `resolver` is not called in the next tick; it is called immediately.
if (methodWasNotInvokedAsAResultOfExplicitUserAction()) {
reject(new DOMException("bad stuff", { name: "SecurityError" }));
return;
}
if (documentsModeOfOperationIsStandalone()) {
reject(new DOMException("bad stuff", { name: "NotSupported" }));
return;
}
var info = getWebApplicationsMetadata();
userAgentSpecificChoooser(function (result) {
if (!result) {
reject(new DOMException("bad stuff", { name: "AbortError" }));
return;
}
resolve(undefined);
});
});
} There is no need to artificially queue a task. |
Ok, got ya! I had a feeling that's what you meant. I was not sure about it being in the next tick or not. I think the above is something important to put into the guide. We've gotten really used to waiting for the next tick (unnecessarily) with "queue at task" and I think people are going to continue to fall into that trap with promises (given that they are kinda new and not well understood). |
Awesome, will be sure to do so! |
@marcoscaceres I added some guidance under Maintain a Normal Control Flow and Do Not Queue Needless Tasks. I also created an updated version of your addBookmark function. I wrote the section Rejections Should Be Used For Exceptional Situations with addBookmark in mind. Is the user denying permission really exceptional? Maybe, maybe not, but let me know what you think. |
So I just read through the guidance section. I think your comment #85 (comment) is actually clearer than what the guidance says right now. The So it would be nice to clarify the guidance further. Perhaps "Do Not Queue Needless Tasks" can explicitly mention that argument validation should be synchronous, even though the user experiences the promise rejection asynchronously. Also/alternatively, the "addBookmark" example might explicitly state, "Note that argument validation and fetching the web application's metadata is done synchronously, even though notification is asynchronous. This prevents races with another task which might mutate the arguments or application metadata." |
Thanks for the feedback @cscott. The guide has largely moved to w3ctag/promises-guide, and we are discussing similar issues at w3ctag/promises-guide#7 |
Usually, when we write async stuff in specs we do something like this:
In the examples given in the spec writing guide, step 2 is omitted. Is this on purpose? Or is doing something like 2. wrong and specs should be written differently?
The text was updated successfully, but these errors were encountered: